messageTray: Let the tray decide whether to show a banner

Always use the same code path to add new messages to a source, and
let the `MessageTray` decide whether it shows a banner.

Part-of: <https://gitlab.gnome.org/GNOME/gnome-shell/-/merge_requests/3173>

diff --git a/js/ui/components/autorunManager.js b/js/ui/components/autorunManager.js
index 7e53f8396ba85df8ed50dbaa5caa9a9310ace711..1b8f718952d4fa9d63c7554df92d90ba93dce8c5 100644 (file)
--- a/js/ui/components/autorunManager.js
+++ b/js/ui/components/autorunManager.js
@@ -203,7 +203,7 @@ class AutorunDispatcher {
         });
         notification.connect('destroy', () => this._notifications.delete(mount));
         this._notifications.set(mount, notification);
-        source.showNotification(notification);
+        source.addNotification(notification);
     }
 
     addMount(mount, apps, contentTypes) {

diff --git a/js/ui/components/networkAgent.js b/js/ui/components/networkAgent.js
index a5d6035648143224f527153bb94a800bad016b78..bab13b184d0655883eaa991d22a38da446b55a15 100644 (file)
--- a/js/ui/components/networkAgent.js
+++ b/js/ui/components/networkAgent.js
@@ -797,7 +797,7 @@ class NetworkAgent {
             delete this._notifications[requestId];
         });
 
-        source.showNotification(notification);
+        source.addNotification(notification);
     }
 
     _newRequest(agent, requestId, connection, settingName, hints, flags) {

diff --git a/js/ui/extensionSystem.js b/js/ui/extensionSystem.js
index 76ae58ff4e4b91b09d5dba045ea33ce50fae306f..a74e08bac219064e4ef3e03e388a99bc551e0741 100644 (file)
--- a/js/ui/extensionSystem.js
+++ b/js/ui/extensionSystem.js
@@ -354,7 +354,7 @@ export class ExtensionManager extends Signals.EventEmitter {
                 _('Extension updates are ready to be installed.'));
             notification.connect('activated',
                 () => source.open());
-            source.showNotification(notification);
+            source.addNotification(notification);
         }
     }
 

diff --git a/js/ui/main.js b/js/ui/main.js
index cfb6d8735d78a9d3023c082e5c41f0166b7de29e..a090a76ae3067b6680e35ebcabf897b9d77775a1 100644 (file)
--- a/js/ui/main.js
+++ b/js/ui/main.js
@@ -280,7 +280,7 @@ async function _initializeUI() {
         notification.addAction(_('Undo'),
             () => (global.context.unsafe_mode = false));
         notification.setTransient(true);
-        source.showNotification(notification);
+        source.addNotification(notification);
     });
 
     // Provide the bus object for gnome-session to
@@ -616,7 +616,7 @@ export function notify(msg, details) {
     const source = MessageTray.getSystemSource();
     let notification = new MessageTray.Notification(source, msg, details);
     notification.setTransient(true);
-    source.showNotification(notification);
+    source.addNotification(notification);
 }
 
 /**

diff --git a/js/ui/messageTray.js b/js/ui/messageTray.js
index a8732e6e07de60202bcd6432aa8910640f3a8a2c..f9c70479bf9729242423a5b8ddc76afe7dd95ca0 100644 (file)
--- a/js/ui/messageTray.js
+++ b/js/ui/messageTray.js
@@ -590,7 +590,7 @@ export const Source = GObject.registerClass({
         'destroy': {param_types: [GObject.TYPE_UINT]},
         'notification-added': {param_types: [Notification.$gtype]},
         'notification-removed': {param_types: [Notification.$gtype]},
-        'notification-show': {param_types: [Notification.$gtype]},
+        'notification-request-banner': {param_types: [Notification.$gtype]},
     },
 }, class Source extends MessageList.Source {
     constructor(params) {
@@ -647,7 +647,7 @@ export const Source = GObject.registerClass({
             this.destroy();
     }
 
-    pushNotification(notification) {
+    addNotification(notification) {
         if (this.notifications.includes(notification))
             return;
 
@@ -655,22 +655,17 @@ export const Source = GObject.registerClass({
             this.notifications.shift().destroy(NotificationDestroyedReason.EXPIRED);
 
         notification.connect('destroy', this._onNotificationDestroy.bind(this));
-        notification.connect('notify::acknowledged', this.countUpdated.bind(this));
-        this.notifications.push(notification);
-        this.emit('notification-added', notification);
-
-        this.countUpdated();
-    }
+        notification.connect('notify::acknowledged', () => {
+            this.countUpdated();
 
-    showNotification(notification) {
-        notification.acknowledged = false;
-        this.pushNotification(notification);
-
-        if (notification.urgency === Urgency.LOW)
-            return;
+            // If acknowledged was set to false try to show the notification again
+            if (!notification.acknowledged)
+                this.emit('notification-request-banner', notification);
+        });
+        this.notifications.push(notification);
 
-        if (this.policy.showBanners || notification.urgency === Urgency.CRITICAL)
-            this.emit('notification-show', notification);
+        this.emit('notification-added', notification);
+        this.emit('notification-request-banner', notification);
     }
 
     destroy(reason) {
@@ -876,7 +871,7 @@ export const MessageTray = GObject.registerClass({
         this._sources.add(source);
 
         source.connectObject(
-            'notification-show', this._onNotificationShow.bind(this),
+            'notification-request-banner', this._onNotificationRequestBanner.bind(this),
             'notification-removed', this._onNotificationRemoved.bind(this),
             'destroy', () => this._removeSource(source), this);
 
@@ -923,7 +918,17 @@ export const MessageTray = GObject.registerClass({
         }
     }
 
-    _onNotificationShow(_source, notification) {
+    _onNotificationRequestBanner(_source, notification) {
+        // We never display a banner for already acknowledged notifications
+        if (notification.acknowledged)
+            return;
+
+        if (notification.urgency === Urgency.LOW)
+            return;
+
+        if (!notification.source.policy.showBanners && notification.urgency !== Urgency.CRITICAL)
+            return;
+
         if (this._notification === notification) {
             // If a notification that is being shown is updated, we update
             // how it is shown and extend the time until it auto-hides.

diff --git a/js/ui/notificationDaemon.js b/js/ui/notificationDaemon.js
index a8c03ac11c3c8fb3906bf44fca8f7ecb3892d1d0..ed3f587202fb45ca406b2ea53790153e5e1578b1 100644 (file)
--- a/js/ui/notificationDaemon.js
+++ b/js/ui/notificationDaemon.js
@@ -200,6 +200,7 @@ class FdoNotificationDaemon {
         const soundFile = 'sound-file' in hints
             ? Gio.File.new_for_path(hints['sound-file']) : null;
 
+        notification.acknowledged = false;
         notification.update(summary, body, {
             gicon,
             bannerMarkup: true,
@@ -355,10 +356,12 @@ class FdoNotificationDaemonSource extends MessageTray.Source {
         }
 
         let tracker = Shell.WindowTracker.get_default();
+        // Acknowledge notifications that are resident and their app has the
+        // current focus so that we don't show a banner.
         if (notification.resident && this.app && tracker.focus_app === this.app)
-            this.pushNotification(notification);
-        else
-            this.showNotification(notification);
+            notification.acknowledged = true;
+
+        this.addNotification(notification);
     }
 
     open() {
@@ -402,9 +405,10 @@ const PRIORITY_URGENCY_MAP = {
 
 const GtkNotificationDaemonNotification = GObject.registerClass(
 class GtkNotificationDaemonNotification extends MessageTray.Notification {
-    _init(source, notification) {
+    _init(source, id, notification) {
         super._init(source);
         this._serialized = GLib.Variant.new('a{sv}', notification);
+        this.id = id;
 
         const {
             title,
@@ -499,15 +503,12 @@ class GtkNotificationDaemonAppSource extends MessageTray.Source {
         this._notificationPending = false;
     }
 
-    _createNotification(params) {
-        return new GtkNotificationDaemonNotification(this, params);
-    }
-
     activateAction(actionId, target) {
         const params = target ? GLib.Variant.new('av', [target]) : null;
         this._app.activate_action(actionId, params, 0, -1, null).catch(error => {
             logError(error, `Failed to activate action for ${this._appId}`);
         });
+
         Main.overview.hide();
         Main.panel.closeCalendar();
     }
@@ -518,22 +519,18 @@ class GtkNotificationDaemonAppSource extends MessageTray.Source {
         Main.panel.closeCalendar();
     }
 
-    addNotification(notificationId, notificationParams, showBanner) {
+    addNotification(notification) {
         this._notificationPending = true;
 
-        if (this._notifications[notificationId])
-            this._notifications[notificationId].destroy(MessageTray.NotificationDestroyedReason.REPLACED);
+        this._notifications[notification.id]?.destroy(
+            MessageTray.NotificationDestroyedReason.REPLACED);
 
-        let notification = this._createNotification(notificationParams);
         notification.connect('destroy', () => {
-            delete this._notifications[notificationId];
+            delete this._notifications[notification.id];
         });
-        this._notifications[notificationId] = notification;
+        this._notifications[notification.id] = notification;
 
-        if (showBanner)
-            this.showNotification(notification);
-        else
-            this.pushNotification(notification);
+        super.addNotification(notification);
 
         this._notificationPending = false;
     }
@@ -609,8 +606,13 @@ class GtkNotificationDaemon {
                         throw e;
                     }
 
-                    notifications.forEach(([notificationId, notification]) => {
-                        source.addNotification(notificationId, notification.deepUnpack(), false);
+                    notifications.forEach(([notificationId, notificationPacked]) => {
+                        const notification = new GtkNotificationDaemonNotification(source,
+                            notificationId,
+                            notificationPacked.deepUnpack());
+                        // Acknowledge all stored notification so that we don't show a banner again
+                        notification.acknowledged = true;
+                        source.addNotification(notification);
                     });
                 });
             }
@@ -635,7 +637,7 @@ class GtkNotificationDaemon {
     }
 
     AddNotificationAsync(params, invocation) {
-        let [appId, notificationId, notification] = params;
+        let [appId, notificationId, notificationSerialized] = params;
 
         let source;
         try {
@@ -651,9 +653,12 @@ class GtkNotificationDaemon {
         }
 
         let timestamp = GLib.DateTime.new_now_local().to_unix();
-        notification['timestamp'] = new GLib.Variant('x', timestamp);
+        notificationSerialized['timestamp'] = new GLib.Variant('x', timestamp);
 
-        source.addNotification(notificationId, notification, true);
+        const notification = new GtkNotificationDaemonNotification(source,
+            notificationId,
+            notificationSerialized);
+        source.addNotification(notification);
 
         invocation.return_value(null);
     }

diff --git a/js/ui/overview.js b/js/ui/overview.js
index 2d6c2a4ab57e27b9afc268ede23ed204f88b6bab..32a9df89d51dfc2984cda3f0d4db0fbdb3e003c8 100644 (file)
--- a/js/ui/overview.js
+++ b/js/ui/overview.js
@@ -50,7 +50,7 @@ class ShellInfo {
         if (undoCallback)
             this._notification.addAction(_('Undo'), () => undoCallback());
 
-        source.showNotification(this._notification);
+        source.addNotification(this._notification);
     }
 }
 

diff --git a/js/ui/screenshot.js b/js/ui/screenshot.js
index f78eeacd13a9c4cc0d9a5909503661aef6eb6caf..d19c6e7b915edff13196ad47a75925d91c5435dc 100644 (file)
--- a/js/ui/screenshot.js
+++ b/js/ui/screenshot.js
@@ -2122,7 +2122,7 @@ export const ScreenshotUI = GObject.registerClass({
         }
 
         Main.messageTray.add(source);
-        source.showNotification(notification);
+        source.addNotification(notification);
     }
 
     get screencast_in_progress() {
@@ -2364,7 +2364,7 @@ function _storeScreenshot(bytes, pixbuf) {
 
     notification.setTransient(true);
     Main.messageTray.add(source);
-    source.showNotification(notification);
+    source.addNotification(notification);
 
     return file;
 }

diff --git a/js/ui/shellMountOperation.js b/js/ui/shellMountOperation.js
index b02b65b6ceceff880df9cf9bee6a560f665a5273..9188410a5e342481e00cc2766ea7f12022c34897 100644 (file)
--- a/js/ui/shellMountOperation.js
+++ b/js/ui/shellMountOperation.js
@@ -200,7 +200,7 @@ export class ShellMountOperation {
         this._notification.setTransient(true);
         this._notification.iconName = 'media-removable-symbolic';
         this._notification.connect('destroy', () => delete this._notification);
-        source.showNotification(this._notification);
+        source.addNotification(this._notification);
     }
 
     _showUnmountNotificationDone(message) {

diff --git a/js/ui/status/network.js b/js/ui/status/network.js
index dad668358d4fe5ef9ecc60e315dc3dd9965a4acb..86cfa95f8b059d9c938e2b7bddf58fdb1c232946 100644 (file)
--- a/js/ui/status/network.js
+++ b/js/ui/status/network.js
@@ -2039,7 +2039,7 @@ class Indicator extends SystemIndicator {
         this._notification.connect('destroy',
             () => (this._notification = null));
 
-        source.showNotification(this._notification);
+        source.addNotification(this._notification);
     }
 
     _syncMainConnection() {

diff --git a/js/ui/status/thunderbolt.js b/js/ui/status/thunderbolt.js
index 9e33c21f55c8e1a868ecaa6d7525dcd44c8fc5ab..96d84d368e21e9434759e2a3c2ba9ee234f099a6 100644 (file)
--- a/js/ui/status/thunderbolt.js
+++ b/js/ui/status/thunderbolt.js
@@ -268,7 +268,7 @@ class Indicator extends SystemIndicator {
             const app = Shell.AppSystem.get_default().lookup_app('gnome-thunderbolt-panel.desktop');
             app?.activate();
         });
-        source.showNotification(this._notification);
+        source.addNotification(this._notification);
     }
 
     /* Session callbacks */

diff --git a/js/ui/windowAttentionHandler.js b/js/ui/windowAttentionHandler.js
index 1525dae296d0762d0b98444f5d9331e085222580..ee701394c2c36f0075ed8f094f6b0f764189c81a 100644 (file)
--- a/js/ui/windowAttentionHandler.js
+++ b/js/ui/windowAttentionHandler.js
@@ -45,7 +45,7 @@ export class WindowAttentionHandler {
         });
         notification.setForFeedback(true);
 
-        source.showNotification(notification);
+        source.addNotification(notification);
 
         window.connectObject('notify::title', () => {
             [title, banner] = this._getTitleAndBanner(app, window);

diff --git a/tests/shell/basic.js b/tests/shell/basic.js
index e5bd8c18786ffc6cb38e761ef83c0b5e559c2d21..5310a31b2efc5788bf12052b5f2459717016b088 100644 (file)
--- a/tests/shell/basic.js
+++ b/tests/shell/basic.js
@@ -53,12 +53,12 @@ export async function run() {
     const source = new MessageTray.getSystemSource();
 
     Scripting.scriptEvent('notificationShowStart');
-    source.connect('notification-show',
+    source.connect('notification-request-banner',
         () => Scripting.scriptEvent('notificationShowDone'));
 
     const notification = new MessageTray.Notification(source,
         'A test notification');
-    source.showNotification(notification);
+    source.addNotification(notification);
     await Scripting.sleep(400);
 
     console.debug('Show date menu');